-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
🌱 clean up samples by removing deprecated options #2741
🌱 clean up samples by removing deprecated options #2741
Conversation
30cc892
to
196632b
Compare
196632b
to
77c961f
Compare
/test pull-kubebuilder-e2e-k8s-1-18-20 |
@@ -123,12 +119,9 @@ build_kb | |||
|
|||
# Project version 2 uses plugin go/v2 (default). | |||
scaffold_test_project project-v2 --project-version=2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we remove this one as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not think so.
Why? we need to ensure in the reviews that changes are not made in this scaffold.
By keeping it, we can easily check that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense
# Project version 3 (default) uses plugin go/v3 (default). | ||
scaffold_test_project project-v3 | ||
scaffold_test_project project-v3-multigroup | ||
scaffold_test_project project-v3-addon --plugins="go/v3,declarative" | ||
scaffold_test_project project-v3-config --component-config | ||
scaffold_test_project project-v3-v1beta1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with removing the v2 projects since they are being deprecated, but are we also deprecating the v1beta1 support for the go/v3 plugin? If not then I think we should keep this one in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with @everettraven.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we do not want anybody using v1neta1. It does not work since K8s >= 1.22 and the flag is deprecated across all places
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gotcha, then this is good.
@@ -34,5 +34,4 @@ build_kb | |||
tools_k8s_version="1.19.2" | |||
fetch_tools | |||
test_project project-v2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we remove this one as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nops, since we will keep the sample
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: camilamacedo86, everettraven, rashmigottipati The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
- remove samples with add-on and declarative for project-v2 - remove samples for v1beta1 CRDs and WebHooks - no longer using this extra options on the legacy tests :seedling: remove samples with add-on and declarative for project-v2
77c961f
to
5397fba
Compare
New changes are detected. LGTM label has been removed. |
Description
Motivation